Conversation
|
cc @kiljacken for the dylib path reading part :) |
There was a problem hiding this comment.
I'd like to pay some attention to minimizing the crate deps here.
Ideally, the ra_db should not depend only any proc-macro specfic, and just export a trait. It should operate on tt::TokenStream, so a dep on ra_tt is required.
Additionally, Ideally proc_macros and mbe do not depend on each other, and use solely tt (which does not depen on ra_syntax). I think we should also make use that proc_macros dont (transitivellly) depend on ra_syntax.
Not sure if the above plan is feasible, but i'd love to try
crates/ra_db/Cargo.toml
Outdated
| ra_syntax = { path = "../ra_syntax" } | ||
| ra_cfg = { path = "../ra_cfg" } | ||
| ra_prof = { path = "../ra_prof" } | ||
| ra_proc_macro = { path = "../ra_proc_macro" } |
There was a problem hiding this comment.
Hm, I wonder if we can minimize the interface here...
Can we use something like this instead?
struct CrateData {
proc_macros: FxHashMap<SmolStr, Arc<dyn Fn(tt::TokenTree) -> tt::TokenTree>>
}
Ie, define a really minimal proc macro interface directly in the ra_db crate, where the minimal interface is ideally an dyn Fn(), or maybe a named trait.
There was a problem hiding this comment.
I changed to :
#[derive(Debug, Clone)]
pub struct ProcMacro {
pub name: SmolStr,
pub expander: Arc<dyn TokenExpander>,
}
impl Eq for ProcMacro {}
impl PartialEq for ProcMacro { ... }
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct CrateData {
...
pub proc_macro: Vec<ProcMacro>,
}|
Also, thanks @edwin0cheng for splitting changes which are small diff-wise, but are large architecturally, into separate PRs! This is massively helpful! |
In other words, if we export a trait in |
|
Good question! Yeah, I think it would also be good if I think bridging them in project-model might work, but, if the expansion trait is really minimal and only depends on tt, we can just put it in the |
|
I think it is plausible, I could imagine it looks like following: enum ExpandError {
...
}
trait TokenExpander: Debug + Send + Sync + RefUnwindSafe {
fn exander(&self, subtree: &Subtree, attrs: Option<&Subtree>) -> Result<Subtree, ExpandError>;
}The trait bound here is needed in |
|
Cargo interacting code looks reasonable to me 👍 Good job, this is some really cool work! |
e9ca593 to
db162df
Compare
|
@matklad, I removed the dependency for mbe and refactoring a bit for salsa trait constraints. You could review it again. 👍 |
|
bors r+ |
Build succeeded |
Rustup To unblock rust-lang/miri#3688
This PR implemented:
OUTDIRis obtained.ra_proc_macroand implement the foot-work for reading result from external proc-macro expander.ProcMacroClient, which will be responsible to the client side communication to the External process.